Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds Swipe layout #3007

Merged
merged 18 commits into from
Mar 14, 2023
Merged

Adds Swipe layout #3007

merged 18 commits into from
Mar 14, 2023

Conversation

MarcSkovMadsen
Copy link
Collaborator

@MarcSkovMadsen MarcSkovMadsen commented Dec 12, 2021

Addresses #3006

Todo

  • Get it working in edge, chrome and firefox
  • Get it working in notebook.
  • Reference Notebook
  • Gallery Example showing it works with Plots, Maps, Images, Tables
  • Be able to interact with before-after panels. Probably using draggable div. See https://www.w3schools.com/howto/howto_js_draggable.asp
  • Pass tests
  • Get review

Nice to Have

  • Before and after title.
  • Get before/ after Spacers to display initially. They display as soon as the window is resized.

@MarcSkovMadsen
Copy link
Collaborator Author

Hi @philippjfr

Can you give me a hint. The BeforeAfterSlider does not show Spacers before I resize the window. What do I need to do to trigger the to (re-)layout. The problem is that they get an initial height of 0px.

before-after-not-working-with-spacers.mp4

You can run panel serve 'panel/tests/layout/test_before_after_slider.py' --autoreload --show to explore the problem.

@MarcSkovMadsen
Copy link
Collaborator Author

Sliderthumb works

(Sliderthumb needs margin-top in chrome/ edge server. But not jupyter labs for some unknown reason)

Firefox Server

image

Edge Server

image

Chrome Server

image

Firefox Jupyterlab

image

Edge Jupyterlab

image

Chrome Jupyterlab

image

@MarcSkovMadsen
Copy link
Collaborator Author

I've added a notebook to the gallery with more examples

before-after-slider3-speedup.mp4

@MarcSkovMadsen
Copy link
Collaborator Author

I've refactored to use a draggable div instead of a range input. Before the range input was in front of everything making the before and after panels inactive. Now you can get tooltips on plots and edit the Tabulator tables.

@philippjfr
Copy link
Member

philippjfr commented Dec 18, 2021

This is looking great! Will review next week. Would love to hear your thoughts on before/after as the API vs making it a ListLike limited to two items.

@MarcSkovMadsen
Copy link
Collaborator Author

Listlike is fine with me. I have a plan to contribute splitter.js based layout also. They are sort of related. So think similar api would be nice.

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Dec 19, 2021

I spent a lot of time trying to add a before and an after title in the top left and right corners.

But as soon as I put those in the HoloViews plot no longer wanted to find its width upfront. It looked like this

image

Try adding something like below in the _template and it will look like above with panel serve --autoreload examples/gallery/layout/BeforeAfterSliderExamples.ipynb.

<div style="width: 100%"><h3 style="position:relative;float:left;top:-35px">After</h3><h3 style="position:relative;float:right;top:-35px">Before</h3></div>

I believe its a natural thing to add. But maybe its ok to leave out for now.

image

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Dec 19, 2021

Hi @philippjfr

I will leave it as is for now waiting for your review. I would not know how to refactor into ListLike so please do and/ or provide examples. I would not know how you would like to restrict listlike to only 2 elements. And especially I would not know how to replace ${before} with a reference to the first item in the list.

If you provide some suggestions I will finalize. Thanks.

@MarcSkovMadsen
Copy link
Collaborator Author

"""
}

if __name__.startswith("bokeh"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The module itself should not have a test in it.

@philippjfr
Copy link
Member

I will leave it as is for now waiting for your review. I would not know how to refactor into ListLike so please do and/ or provide examples.

Okay I'll take a look, I'll probably leave before and after as properties so you can still do layout.before = ...

@jbednar
Copy link
Member

jbednar commented Dec 23, 2021

I'm eager to see this appear! @MarcSkovMadsen , you probably don't know, but the Datashader logo is an abstraction of a imagined before/after comparison between a subsampled and a datashaded plot. It only exists in my head, but I've always wanted to make it an actual example, and soon I will be able to!

@philippjfr
Copy link
Member

@MarcSkovMadsen I've partially rewritten this but it still suffers from some issues with responsive contents. You had a partial fix for that simply by setting the width/height of the contents but that was only a partial fix because it would not work for layouts containing multiple components. I'm still looking for a general solution.

@MarcSkovMadsen
Copy link
Collaborator Author

Ok. Thanks.

If you want me to search for a solution please provide a minimum reproducible example such that I can understand and work with the issue.

@GeoVizNow
Copy link

GeoVizNow commented Jan 6, 2022

Hi,
thanks for the good work!
I was trying to test what I found on the 'before-after-slider' branch.

Below is the code used..

Some imports:

from IPython.core.display import display, HTML
import param
import holoviews as hv
from holoviews import opts
import panel as pn
from panel.reactive import ReactiveHTML
from panel.layout.base import ListLike
import numpy as np
hv.extension('bokeh')
pn.extension(sizing_mode="stretch_width")

Class (as I saw it implemented already)

class BeforeAfterSlider(ListLike, ReactiveHTML):
    """
    The BeforeAfterSlider layout enables you to quickly compare two
    panels layed out on top of each other with a part of the *before*
    panel shown on one side of a slider and a part of the
    *after* panel shown on the other side."""

    objects = param.List(default=[], bounds=(0, 2), doc="""
        The list of child objects that make up the layout.""", precedence=-1)

    slider_width = param.Integer(default=12, bounds=(0, 25), doc="""
        The width of the slider in pixels""")

    slider_color = param.Color(default="silver", doc="""
        The color of the slider""")

    value = param.Integer(50, bounds=(0, 100), doc="""
        The percentage of the *after* panel to show.""")

    _before = param.Parameter()

    _after = param.Parameter()

    _template = """
    <div id="container" class="before-after-container">
      <div id="before" class="outer">
        <div id="before-inner" class="inner">${_before}</div>
      </div>
      <div id="after" class="outer" style="overflow: hidden;">
        <div id="after-inner" class="inner">${_after}</div>
      </div>
      <div id="slider" class="slider" onmousedown="${script('drag')}"
           style="background: ${slider_color}; width: ${slider_width}px;">
      </div>
    </div>
    """

    _scripts = {
        'render': """
          set_size(container, model)
          self.adjustSlider()
        """,
        'after_layout': """
          self.value()
        """,
        'drag': """
          function endDrag() {
             document.removeEventListener('mouseup', endDrag);
             document.removeEventListener('mousemove', handleDrag);
           }
           function handleDrag(e) {
             e = e || window.event;
             e.preventDefault();
             current = e.clientX
             start = view.el.getBoundingClientRect().left
             value = parseInt(((current-start)/ container.clientWidth)*100)
             data.value = Math.max(0, Math.min(value, 100))
           }
           let e = event || window.event;
           e.preventDefault();
           document.addEventListener('mouseup', endDrag);
           document.addEventListener('mousemove', handleDrag);
        """,
        'value': """
           after.style.width = `calc(${data.value}% + 5px)`
           self.adjustSlider()
        """,
        'slider_width': """
           self.adjustSlider()
        """,
        'adjustSlider': """
           halfWidth = parseInt(data.slider_width/2)
           slider.style.marginLeft = `calc(${data.value}% + 5px - ${halfWidth}px)`
        """
    }

    def __init__(self, *objects, **params):
        if 'objects' in params and objects:
            raise ValueError(
                "Either supply objects as an positional argument or "
                "as a keyword argument, not both."
            )
        objects = params.pop('objects', objects)
        super().__init__(objects=list(objects), **params)

    @param.depends('objects', watch=True, on_init=True)
    def _update_layout(self):
        self._before = self.before
        self._after = self.after

    @property
    def before(self):
        return self[0] if len(self) else None

    @before.setter
    def before(self, before):
        self[0] = before

    @property
    def after(self):
        return self[1] if len(self) > 1 else None

    @after.setter
    def after(self, after):
        self[1] = after

Example image genereation taken from holoviews gallery:

ls = np.linspace(0, 10, 200)
xx, yy = np.meshgrid(ls, ls)

bounds=(-1,-1,1,1)   # Coordinate system: (left, bottom, right, top)
bounds=(0,0,200,200)

img = hv.Image(np.sin(xx)*np.cos(yy), bounds=bounds).opts(frame_height=400, data_aspect=1)
img2 = hv.Image(np.sin(xx)*np.cos(yy), bounds=bounds).opts(cmap='Reds', frame_height=400, data_aspect=1)

Showing the BeforeAfterSlider instance:

before_after = BeforeAfterSlider(img, img2, value=50, slider_width=3, slider_color='magenta', height=400)
before_after

swipe-widget-example

Amazing that you have already worked on this feature!. Not sure of the value of my tes, but I noticed a few things that I wanted to highlight:

  • the vertical line is lagging behind the actual moving of the delimiting line for the before and after objects in the above example (a detail)
  • it would be very good if the zoom and pan from the toolbar would apply to both before and after objects as together, i.e. would be good to be able to zoom and pan both e.g. images and modify the swipe line in several iterations without a reset of the container (this could be important)
  • would there be a way top limit the swipe line to the extent of the frame for say a holoviews image? (in above example the line is overplotting the axis labels and tics a bit, not a big worry though)

I am really eager to see this widget apply into geoviews too as I think would be possible given the path of implementation.

For what it's worth I found one more source of inspiration that might be useful (link here: https://www.w3schools.com/howto/howto_js_image_comparison.asp)

@jbednar
Copy link
Member

jbednar commented Jan 7, 2022

@GeoVizNow , I think what you're asking for is something more tailored to Bokeh plots in particular rather than this very generic Panel-level approach, so that it would respect the bokeh zoom and axes. Those issues seem out of scope for this particular feature, but would be appropriate for such a feature implemented at the Bokeh level. We've done something like that using the techniques at http://holoviews.org/user_guide/Custom_Interactivity.html, but I can't find the code for it just now; it was much more specific to a particular type of plot but did respect the Bokeh plot constraints because it was implemented at the bokeh plot level.

@mattpap
Copy link
Collaborator

mattpap commented Aug 2, 2022

As an FYI, in PR bokeh/bokeh#12083 (scheduled for bokeh 3.1) I implemented various interactive shapes, including spans, with the intention of having in-canvas UI controls, especially sliders. Interactive spans (movable, with bounds, etc.), together with improvements to clipping (e.g. customizable clip regions per renderer or a group of renderers), could be used to implement the proposed in this PR feature idiomatically and efficiently on top of bokeh 3.1.

@philippjfr philippjfr force-pushed the before-after-slider branch from 260d926 to f5e8ffc Compare March 13, 2023 13:13
@philippjfr
Copy link
Member

Finally found a solution here using the clip-path CSS property.

@philippjfr philippjfr added this to the v1.0.0 milestone Mar 13, 2023
@philippjfr
Copy link
Member

pre-commit.ci autofix

@codecov
Copy link

codecov bot commented Mar 13, 2023

Codecov Report

Merging #3007 (4cc8851) into main (fc23efe) will decrease coverage by 0.08%.
The diff coverage is 96.66%.

@@            Coverage Diff             @@
##             main    #3007      +/-   ##
==========================================
- Coverage   83.36%   83.29%   -0.08%     
==========================================
  Files         247      250       +3     
  Lines       36309    36521     +212     
==========================================
+ Hits        30270    30420     +150     
- Misses       6039     6101      +62     
Flag Coverage Δ
ui-tests 39.10% <89.07%> (-0.14%) ⬇️
unitexamples-tests 73.51% <44.16%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
panel/__init__.py 100.00% <ø> (ø)
panel/layout/swipe.py 90.47% <90.47%> (ø)
panel/layout/__init__.py 100.00% <100.00%> (ø)
panel/tests/layout/test_swipe.py 100.00% <100.00%> (ø)
panel/tests/ui/layout/test_swipe.py 100.00% <100.00%> (ø)

... and 15 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@philippjfr philippjfr changed the title Adds BeforeAfterSlider Adds Swipe layout Mar 14, 2023
@philippjfr philippjfr merged commit 6ad3b98 into main Mar 14, 2023
@philippjfr philippjfr deleted the before-after-slider branch March 14, 2023 14:18
@GeoVizNow
Copy link

GeoVizNow commented Mar 18, 2023

Amazing to see this in, really great work (though I do not know all that lays behind)!!
It works really nicely! Even zooming on overlaid holoviews images works in sync. with both images.
Notice that the zoom box is only active on the top side image, but works in sync on both.
Was trying an example with the following installed versions:

  • holoviews 1.16.0a2
  • panel 1.0.0b2
  • bokeh 3.1.0
### Test swipe layout
import holoviews as hv
import panel as pn
import xarray as xr
import hvplot.xarray
pn.extension(sizing_mode='stretch_width')

# Example code taken from https://hvplot.holoviz.org/reference/xarray/image.html
ds = xr.tutorial.open_dataset('air_temperature')

# time = '2014-01-01'
data_2013_01 = ds.sel(time='2013-01-01').mean('time') - 273  # convert to celcius
data_2014_01 = ds.sel(time='2014-01-01').mean('time') - 273  # convert to celcius

img_opts = dict(cmap='gray', frame_height=400, frame_width=400)
pn.layout.Swipe(data_2013_01.hvplot.image(title='2013'), data_2014_01.hvplot.image(title='2014'), value=49, slider_color='white')

Couple of more things noted:

  • would have been amazing to limit the height of the slider to the height of the frame if it being a holoviews element, but do not know if possible. If setting the slider_color to same as plot backgroud it is of less importance (shown in animation)
  • if panel sizing_mode='stretch_width' then the 'value' of slider seems to reference the full page width while the bokeh image is not (an observation, maybe as one would expect?).
  • the hover tooltips seem to be clipped by the slider. Would have been bonus if they are not clipped.
  • the box zoom is only visually traced with stippled black rectangle on the top side 'after' image, but works in both.

2023-03-18_20-57-00

Again, really great to see this being possible!! Thanks @philippjfr, @MarcSkovMadsen, @jbednar, @mattpap and others involved!!

@GeoVizNow
Copy link

One more note. I added a title to each holoviews element through hvpot. Was there any thought of title for each 'before' , 'after' object too? The way added in above example makes it align badly for the inconveniently for the 'after' image..

@GeoVizNow
Copy link

By the way, is it ok to discuss this here even though the issue is merged or do you prefer a different place?

@philippjfr
Copy link
Member

A new issue about any suggestions for improvement would be good. Titles are definitely a good one.

@GeoVizNow
Copy link

@philippjfr, thanks will create a new issue with one or more enhancement proposals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants